Proposed changes for multi-testnet faucet option #32
Conversation
| @@ -85,6 +87,11 @@ function startServer () { | |||
| async function handleRequest (req, res) { | |||
| try { | |||
| // parse address | |||
| // can parse for network here. | |||
There was a problem hiding this comment.
server must have an explicit allowlist (not a denylist) for network ids it permits to send from. that faucet occasionally accidentally holds real world assets on mainnet and bsc
There was a problem hiding this comment.
@kumavis Got it, what does that mean for the parsing here?
| // can either be json object or | ||
| // inserted in a combined rawdata string with address, and then parsed out here | ||
| // depending on what the network is, make sure ethQuery engine knows when | ||
| // testing balance and sending transaction |
| const balance = await ethQuery.getBalance(targetAddress, 'pending') | ||
| const balanceTooFull = balance.gt(MAX_BALANCE) | ||
| if (balanceTooFull) { | ||
| console.log(`${requestorMessage} - already has too much ether`) | ||
| return didError(res, new Error('User is greedy - already has too much ether')) | ||
| } | ||
| // send value | ||
| // need ot change network here as well: |
There was a problem hiding this comment.
I don't think we're currently doing eip155 sigs here, we should fix that
There was a problem hiding this comment.
also going to need infura api keys for each network
@kumavis I don't think we will if we're using Infura the way it normally is. As you can see here, you can change the Ethereum network from an Infura API simply by changing the subdomain:
The one issue I'm having is how to dynamically change rpcUrl of the engine based on user's request. The relevant code is here:
Do you have any ideas of how to do this?
| @@ -36,6 +36,8 @@ console.log('Acting as faucet for address:', config.address) | |||
| // | |||
|
|
|||
| // ProviderEngine based caching layer, with fallback to geth | |||
| // | |||
| // need to be able to dynamically change rpcOrigin based on network requesting funds | |||
There was a problem hiding this comment.
the provider stack is quite, old it should be replace by https://github.com/MetaMask/eth-json-rpc-middleware and the json-rpc-engine stack
There was a problem hiding this comment.
the provider stack is quite, old it should be replace by https://github.com/MetaMask/eth-json-rpc-middleware and the json-rpc-engine stack
@kumavis Will that change the syntax here much?
| @@ -232,16 +253,18 @@ async function getEther () { | |||
| if (!account) return | |||
|
|
|||
| var uri = `${window.location.href}v0/request` | |||
| var data = account | |||
| var data = { | |||
There was a problem hiding this comment.
may have to json serialize this first. verify that the fetch api supports providing an object
There was a problem hiding this comment.
@kumavis just a quick search makes it seem like I may have to use JSON.stringify(), does that sound right?
|
you should also checkout https://github.com/MetaMask/test-dapp/ and see if it meets your needs |
|
also going to need infura api keys for each network |
Hey there, two main changes:
I'm not familiar enough with the codebase or ethQuery to make the best decision about this, but will message you on slack as well.
The other thing I'm stuck on is the best way to encode which network the user wants when sending the
POSTrequest -- it could either be stuck onto the front of the rawdata string that's parsed, or the webapp can format thereqinto a JSON. but I think parsing a JSON on the server side might be a little too weird, but you would know best.